Conversation
alexhancock
force-pushed
the
alexhancock/remove-settings-v1
branch
from
6a8c892 to
9877bdd
Compare
zanesq
left a comment
zanesq
left a comment
There was a problem hiding this comment.
code LGTM! I tested locally and ran the e2e tests and everything seems to still work fine.
(caveat) I've only used settings v2 since I've been on the team so maybe not the best to understand what functionality use to be in settings v1 and whats not needed anymore.
I added a few comments for clarification but feel free to ignore them if it's known changes with v1 -> v2
| import RecipeExpandableInfo from './RecipeExpandableInfo'; | ||
| import { ScheduleFromRecipeModal } from './schedule/ScheduleFromRecipeModal'; | ||
| // import ExtensionList from './settings_v2/extensions/subcomponents/ExtensionList'; | ||
| // import ExtensionList from './settings/extensions/subcomponents/ExtensionList'; |
There was a problem hiding this comment.
Can probably remove this comment?
| viewOptions={viewOptions as SettingsViewOptions} | ||
| /> | ||
| )} | ||
| {view === 'moreModels' && ( |
There was a problem hiding this comment.
Do we not need these anymore?
| loadProviderDetails(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [currentModel]); | ||
| }, []); |
There was a problem hiding this comment.
be careful removing these deps, I think we might want the model to refresh here when it changes
There was a problem hiding this comment.
I added back the eslint disable comment for linting purposes
| @@ -102,9 +100,6 @@ export const AddModelModal = ({ onClose, setView }: AddModelModalProps) => { | |||
| writeToConfig: upsert, | |||
| }); | |||
|
|
|||
| // Update the model context | |||
There was a problem hiding this comment.
Ah maybe its not needed anymore?
| </ErrorBoundary> | ||
| </ActiveKeysProvider> | ||
| </ModelProvider> | ||
| <ErrorBoundary> |
There was a problem hiding this comment.
Guess removing these was related to not needing model refresh?
alexhancock
force-pushed
the
alexhancock/remove-settings-v1
branch
from
2daaaba to
9a78542
Compare
…et model/provider for display
alexhancock
force-pushed
the
alexhancock/remove-settings-v1
branch
from
0e6d2e7 to
7a45e70
Compare
* main: fix: pr comment build cli workflow (#2774) hotfix: don't always run prompt (#2773) Lifei/test workflow (#2772) chore: use hermit to install node, rust and protoc (#2766) Feat: Refined the documentation for Goose (#2751) mcp(developer): add fallback on .gitignore if no .gooseignore is present (#2661) cli(ux): Show active context length in CLI (#2315) cli(config): Add GOOSE_CONTEXT_STRATEGY setting (#2666) fix: new models have different messages for context length exceeded (#2763) fix: increase limit for direct to disk for performance (#2762) Revert "chore: use hermit in goose" (#2759) alexhancock/remove-settings-v1 (#2744) blog: Democratizing Detection Engineering at Block with Goose and Panther MCP (#2746)
Co-authored-by: Zane Staggs <zane@squareup.com>
2 participants
Cleanup!
settings_v2intosettingssettings